Add call to HandleParticlesAtBoundaries in WarpXEvolve.cpp to prevent oob segfaults #5136
Conversation
|
Looks like the Also, the |
|
I think one could reset these checksums. The test changes the order of particles that go into the checksum by redistributing them. Long floating point sums are not commutative. |
|
How could I go about doing that? |
Source/Evolve/WarpXEvolve.cpp
Outdated
| @@ -176,6 +176,8 @@ WarpX::Evolve (int numsteps) | |||
| if (evolve_scheme == EvolveScheme::Explicit) { | |||
There was a problem hiding this comment.
I would suggest a more correct way of fixing this is to move this whole block of code to the end of the time step after the electrostatic field calculation (and before the diagnostics). This is suggested in the ancient PR #1751. This fixes two things. At that point the particle boundary conditions would have been handled anyway, and also the fields updated to the correct time level for the velocity push.
There was a problem hiding this comment.
Ok, that seems reasonable.
There was a problem hiding this comment.
Unfortunately, this causes many CI tests to fail. I can work through the tests and update the benchmarks, making sure that nothing is actually broken. This will take some time.
Co-authored-by: David Grote <[email protected]>
|
Closing this for now. May create a fresh PR in the future. |
Fixes bug found here #5065 and partially addresses #5113
This PR just adds a call to HandleParticlesAtBoundaries to the synchronization step part of WarpXEvolve.cpp to prevent unhelpful memory access errors when particles travel too far in one step.
Before, this error would be thrown:
Now, no error is thrown and the simulation finishes successfully.
Ideally, we would still like to warn users about CFL violations in ES simulations, or adapt the timestep to account for a user-specified CFL number (which may be greater than 1 for semi-implicit schemes). I'll keep looking at that.